From 8fa7e68a7527535f56b630994832e50978b6dc5b Mon Sep 17 00:00:00 2001 From: Timo Sirainen Date: Mon, 23 Feb 2026 19:54:40 +0200 Subject: [PATCH] [PATCH 08/24] auth: Rewrite ldap_escape() with a unit test Gbp-Pq: Name CVE-2026-24031-27860-5.patch --- src/auth/Makefile.am | 2 + src/auth/db-ldap.c | 89 +++++++++++++++++++++++++++++++++++--------- src/auth/db-ldap.h | 1 + src/auth/test-auth.h | 1 + src/auth/test-ldap.c | 39 +++++++++++++++++++ src/auth/test-main.c | 3 ++ 6 files changed, 117 insertions(+), 18 deletions(-) create mode 100644 src/auth/test-ldap.c diff --git a/src/auth/Makefile.am b/src/auth/Makefile.am index 1d4b5d0..d3745b8 100644 --- a/src/auth/Makefile.am +++ b/src/auth/Makefile.am @@ -178,6 +178,7 @@ libauthdb_ldap_la_LDFLAGS = -module -avoid-version -shared libauthdb_ldap_la_LIBADD = $(LIBDOVECOT_LDAP) $(LDAP_LIBS) libauthdb_ldap_la_CPPFLAGS = $(AM_CPPFLAGS) -DPLUGIN_BUILD libauthdb_ldap_la_SOURCES = $(ldap_sources) +test_auth_ldadd_plugins += libauthdb_ldap.la else auth_libs += $(LIBDOVECOT_LDAP) endif @@ -225,6 +226,7 @@ test_auth_SOURCES = \ test-auth-request-var-expand.c \ test-auth-request-fields.c \ test-username-filter.c \ + test-ldap.c \ test-lua.c \ test-mock.c \ test-main.c diff --git a/src/auth/db-ldap.c b/src/auth/db-ldap.c index 9dcebed..8eda924 100644 --- a/src/auth/db-ldap.c +++ b/src/auth/db-ldap.c @@ -13,6 +13,7 @@ #include "str.h" #include "strescape.h" #include "time-util.h" +#include "unichar.h" #include "env-util.h" #include "settings.h" #include "ssl-settings.h" @@ -54,8 +55,6 @@ #define DB_LDAP_REQUEST_MAX_ATTEMPT_COUNT 3 #define DB_LDAP_ATTR_DN "~dn" -static const char *LDAP_ESCAPE_CHARS = "*,\\#+<>;\"()= "; - struct db_ldap_result { int refcount; LDAPMessage *msg; @@ -1139,26 +1138,80 @@ void db_ldap_get_attribute_names(pool_t pool, *sensitive_r = array_front(&sensitive_attr_names); } -#define IS_LDAP_ESCAPED_CHAR(c) \ - ((((unsigned char)(c)) & 0x80) != 0 || strchr(LDAP_ESCAPE_CHARS, (c)) != NULL) - -const char *ldap_escape(const char *str, - void *context ATTR_UNUSED) +const char *ldap_escape(const char *input, void *context ATTR_UNUSED) { - string_t *ret = NULL; + /* This function escapes both LDAP filters and LDAP DNs. This works, + because both allow using the method of escaping any characters. + However, this isn't really recommended, since apparently some LDAP + servers and perhaps other tools don't handle unnecessary escaping + correctly. + + Another issue is what to do with invalid UTF-8. LDAP filter RFC + suggests just escaping invalid UTF-8. LDAP DN RFC doesn't at least + explicitly say that. It instead seems to imply that the string is + just expected to be valid UTF-8. Or perhaps to use #hex-string + encoding, which isn't compatible with LDAP filters. Just to be safe, + we'll replace invalid UTF-8 input with the UTF-8 replacement + character. + + So this function isn't perhaps the most standards compliant one, but + we don't really care, since this escaping is mainly intended to + prevent untrusted username input from breaking out of the filter or + DN. Valid usernames are unlikely to require escaping or to be + invalid UTF-8. */ + + /* Convert invalid UTF-8 characters to replacement characters. */ + size_t input_len = strlen(input); + string_t *str = t_str_new(64); + if (!uni_utf8_get_valid_data((const unsigned char *)input, + input_len, str)) { + /* Input was not valid UTF-8 */ + input = t_strdup(str_c(str)); + input_len = str_len(str); + str_truncate(str, 0); + } - for (const char *p = str; *p != '\0'; p++) { - if (IS_LDAP_ESCAPED_CHAR(*p)) { - if (ret == NULL) { - ret = t_str_new((size_t) (p - str) + 64); - str_append_data(ret, str, (size_t) (p - str)); - } - str_printfa(ret, "\\%02X", (unsigned char)*p); - } else if (ret != NULL) - str_append_c(ret, *p); + const char *escape_chars = + /* control characters */ + "\x01\x02\x03\x04\x05\x06\x07\x08" + "\x09\x0a\x0b\x0c\x0d\x0e\x0f\x10" + "\x11\x12\x03\x14\x15\x16\x17\x18" + "\x19\x1a\x1b\x1c\x1d\x1e\x1f" + /* filter + DN */ + "\\" + /* filter only */ + "*()" + /* DN only (not including leading and trailing chars) */ + ",+<>;\"="; + size_t pos; + + /* check the leading character separately (required by DN) */ + if (input[0] == '#' || input[0] == ' ') + pos = 0; + else { + pos = strcspn(input, escape_chars); + if (pos == input_len) { + /* the last trailing space is escaped + (required by DN) */ + if (pos > 0 && input[pos - 1] == ' ') + pos--; + else + return input; + } } - return ret == NULL ? str : str_c(ret); + do { + str_append_data(str, input, pos); + str_printfa(str, "\\%02x", (unsigned char)input[pos]); + input += pos + 1; + input_len -= pos + 1; + pos = strcspn(input, escape_chars); + /* the last trailing space is escaped (required by DN) */ + if (pos == input_len && pos > 0 && input[pos - 1] == ' ') + pos--; + } while (pos < input_len); + str_append_data(str, input, pos); + return str_c(str); } static bool diff --git a/src/auth/db-ldap.h b/src/auth/db-ldap.h index 7b2f60f..a6aac02 100644 --- a/src/auth/db-ldap.h +++ b/src/auth/db-ldap.h @@ -169,6 +169,7 @@ void db_ldap_connect_delayed(struct ldap_connection *conn); void db_ldap_enable_input(struct ldap_connection *conn, bool enable); +const char *ldap_dn_escape(const char *str, void *context); const char *ldap_escape(const char *str, void *context); const char *ldap_get_error(struct ldap_connection *conn); diff --git a/src/auth/test-auth.h b/src/auth/test-auth.h index 517347a..16e581d 100644 --- a/src/auth/test-auth.h +++ b/src/auth/test-auth.h @@ -14,6 +14,7 @@ void test_auth_request_var_expand(void); void test_auth_request_fields(void); void test_db_dict_parse_cache_key(void); void test_username_filter(void); +void test_db_ldap(void); void test_db_lua(void); struct auth_passdb *passdb_mock(void); void passdb_mock_mod_init(void); diff --git a/src/auth/test-ldap.c b/src/auth/test-ldap.c new file mode 100644 index 0000000..cd9904a --- /dev/null +++ b/src/auth/test-ldap.c @@ -0,0 +1,39 @@ +/* Copyright (c) 2026 Dovecot authors, see the included COPYING file */ + +#include "test-auth.h" + +#ifdef HAVE_LDAP +#include "db-ldap.h" + +static void test_ldap_escape(void) +{ + struct { + const char *input; + const char *output; + } tests[] = { + { "", "" }, + { " ", "\\20" }, + { " ", "\\20\\20" }, + { "foo ", "foo\\20" }, + { ",", "\\2c" }, + { "s p a c e", "s p a c e" }, + { "# start-end#", "\\23 start-end#" }, + { " start-end2 ", "\\20 start-end2 \\20" }, + { "middle:,+\"\\<>;=", "middle:\\2c\\2b\\22\\5c\\3c\\3e\\3b\\3d" }, + { "valid-utf8:\xc3\xb1", "valid-utf8:\xc3\xb1" }, + { "Bad \xFF Characters", "Bad \xEF\xBF\xBD Characters" }, + }; + test_begin("ldap_escape()"); + for (unsigned int i = 0; i < N_ELEMENTS(tests); i++) { + test_assert_strcmp_idx(ldap_escape(tests[i].input, NULL), + tests[i].output, i); + } + test_end(); +} + +void test_db_ldap(void) +{ + test_ldap_escape(); +} + +#endif diff --git a/src/auth/test-main.c b/src/auth/test-main.c index cb05fda..64467c4 100644 --- a/src/auth/test-main.c +++ b/src/auth/test-main.c @@ -24,6 +24,9 @@ int main(int argc, char *argv[]) TEST_NAMED(test_username_filter) #if defined(HAVE_LUA) TEST_NAMED(test_db_lua) +#endif +#if defined(HAVE_LDAP) + TEST_NAMED(test_db_ldap) #endif { NULL, NULL } }; -- 2.30.2